Check for entitlement in DBP agent #2802

Bunn · 2024-05-21T13:58:46Z

Task/Issue URL: https://app.asana.com/0/1201011656765697/1206366654222841/f

Description:
Deactivates the DBP agent if entitlements are invalid

Steps to test this PR:
The main changes here are in DefaultDataBrokerProtectionAgentStopper and DataBrokerProtectionSubscriptionEventHandler which now checks for entitlement.

Start from scratch removing the database and killing all agents you have running rm -rf ~/Library/Group\ Containers/HKE973VLUW.com.duckduckgo.macos.browser.dbp.debug and launchctl list | grep duck | awk '{print $3}' | xargs -n 1 launchctl remove
Subscripe for privacy pro using the staging env (Set the staging env for subscription AND PIR in the debug menu)
Check if you can correctly use the DBP feature
Debug the agent and the browser and check in the console if we're sending the m_mac_dbp_macos_entitlement_valid pixels
Change this to 2 minutes and check if we're correctly checking for entitlements every 2 minutes
Change this to return false (AFAIK there's no other way to simulate this), and validate if we're correctly calling exit(0) in the agent and disableAndDelete in the browser
Check that the agent dies if this validation fails, and no subsequent calls are made

https://app.asana.com/0/1201011656765697/1206366654222841/f

github-actions · 2024-05-21T14:00:44Z

	Warnings
⚠️	PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS against 42264a4

...ataBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentKiller.swift

aataraxiaa

Nice one @Bunn. Overall approach LGTM! Protocols will allow us to test etc, which is great. I know it’s a WIP, but left a few small questions/comments anyway.

DuckDuckGo/DBP/DataBrokerProtectionSubscriptionEventHandler.swift

aataraxiaa · 2024-05-21T16:13:13Z

.../Sources/DataBrokerProtection/Authentication/DataBrokerProtectionEntitlementMonitoring.swift

+extension DataBrokerProtectionEntitlementMonitoring {
+    func start(checkEntitlementFunction: @escaping () async throws -> Bool, callback: @escaping (DataBrokerProtectionEntitlementMonitorResult) -> Void) {
+        start(checkEntitlementFunction: checkEntitlementFunction,
+              intervalInMinutes: DataBrokerProtectionEntitlementMonitor.monitoringInterval,


Minor: I don’t think it will cause issues, but something about the protocol extension referencing the concrete type static variable seems strange, or inverted. Maybe it’s a pattern, unsure, but wanted to highlight. Other options would be to just declare the constant in the function, or if we want every concrete type to have it’s own interval, define monitoringInterval as a protocol requirement.

Declaring the const in the function is the way to go IMO. The static const there is a leftover from some tests. Good call, thanks. I'll fix it

...okerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift

Co-authored-by: Pete Smith <[email protected]>

.../Sources/DataBrokerProtection/Authentication/DataBrokerProtectionEntitlementMonitoring.swift

...okerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift

...taBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentStopper.swift

LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/Logging.swift

aataraxiaa

Had a look at latest changes and they LGTM 👍🏼

testWhenAgentStart_andProfileDoesNotExist_thenActivityIsNotScheduled_andSheduledOpereationsNotRun is not required anymore since this flow is being tested on DataBrokerProtectionAgentStopperTests

...BrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentManagerTests.swift

aataraxiaa

Overall looks good @Bunn . All test steps passed. Left some comments, the main ones relating to tests.

...taBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentStopper.swift

...BrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentManagerTests.swift

...BrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentStopperTests.swift

aataraxiaa

Changes mostly look good 👍🏼, but the added test needs to be changed. See the comment.

aataraxiaa · 2024-05-23T08:30:12Z

...BrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentManagerTests.swift

+            agentFinishedLaunchingExpectation.fulfill()
+        })
+
+        // Then


We are not awaiting the completion of the expectation here, so it’s reaching the asserts immediately, before any of the completion handlers are called. Also, if we want to wait for each completion, we will need multiple expectations.

# Conflicts: # DuckDuckGo/Application/AppDelegate.swift # DuckDuckGo/DBP/DataBrokerProtectionSubscriptionEventHandler.swift

aataraxiaa

Latest changes LGTM 🎉

# By Fernando Bunn (3) and others # Via GitHub * main: Autofill engagement KPIs for pixel reporting (#2806) autofill: don't prefix autofill email pixels with `m.mac.` (#2808) Bump BSK (#2807) Make profile selector optional (#2811) Add History to iOS (updated UI and rollout) (#2770) New autofill save & update password prompt pixels for alignment with iOS (#2801) Add mylife data broker (#2786) Increase test timeout (#2810) Subscription refactoring, BSK update (#2809) Check for entitlement in DBP agent (#2802) move permanent survey card to first position (#2804) # Conflicts: # DuckDuckGo.xcodeproj/project.pbxproj

# By Elle Sullivan (8) and others # Via Elle Sullivan (3) and GitHub (2) * main: (26 commits) Autofill engagement KPIs for pixel reporting (#2806) autofill: don't prefix autofill email pixels with `m.mac.` (#2808) Bump BSK (#2807) Make profile selector optional (#2811) Add History to iOS (updated UI and rollout) (#2770) New autofill save & update password prompt pixels for alignment with iOS (#2801) Add mylife data broker (#2786) Increase test timeout (#2810) Subscription refactoring, BSK update (#2809) Check for entitlement in DBP agent (#2802) move permanent survey card to first position (#2804) Subscription refactoring (#2764) Bump version to 1.89.0 (191) Merge pir stabilization feature branch into main (#2789) Update age params for multiple brokers (#2800) Fix top level navigation blocks (#2792) Adding to the Dock automatically (#2722) Fix lint error Make activity only call completion once all work has actually finished Fix activity scheduler not calling completion ... # Conflicts: # DuckDuckGo.xcodeproj/project.pbxproj # DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved

* main: (33 commits) Remove autofill survey (#2819) Bump version to 1.90.0 (192) Set marketing version to 1.90.0 Update embedded files Privacy Pro survey support (#2816) Update PeopleWhiz Broker Files to use hash id (#2814) Update BSK due to iOS changes. (#2776) Scroll address bar to caret when using arrows (#2799) Privacy Pro macOS quick follow ups (#2813) BSK bump for iOS RMF updates (#2798) Autofill engagement KPIs for pixel reporting (#2806) autofill: don't prefix autofill email pixels with `m.mac.` (#2808) Bump BSK (#2807) Make profile selector optional (#2811) Add History to iOS (updated UI and rollout) (#2770) New autofill save & update password prompt pixels for alignment with iOS (#2801) Add mylife data broker (#2786) Increase test timeout (#2810) Subscription refactoring, BSK update (#2809) Check for entitlement in DBP agent (#2802) ...

WIP: checking for entitlement

a33ad82

https://app.asana.com/0/1201011656765697/1206366654222841/f

Bunn commented May 21, 2024

View reviewed changes

...ataBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentKiller.swift Outdated Show resolved Hide resolved

Bunn added 2 commits May 21, 2024 15:12

Renames

d0a43b1

Rename file

8152027

aataraxiaa reviewed May 21, 2024

View reviewed changes

Bunn and others added 3 commits May 21, 2024 17:43

PR feedback

cf789fe

add comments

a1a17a9

Update DuckDuckGo/DBP/DataBrokerProtectionSubscriptionEventHandler.swift

821112e

Co-authored-by: Pete Smith <[email protected]>

THISISDINOSAUR reviewed May 21, 2024

View reviewed changes

PR feedback

37a4515

aataraxiaa reviewed May 22, 2024

View reviewed changes

Bunn added 5 commits May 22, 2024 11:27

Add pixels for subscriptionEventHandler

e3b5d00

Handle entitlement check in the background agent

4870a05

Add tests

7a950d4

Linter

53d8106

Fix tests

3367dd3

testWhenAgentStart_andProfileDoesNotExist_thenActivityIsNotScheduled_andSheduledOpereationsNotRun is not required anymore since this flow is being tested on DataBrokerProtectionAgentStopperTests

Bunn requested a review from aataraxiaa May 22, 2024 16:47

github-actions bot added the bot: not in app board Added by automation for pull requests with tasks not added to macOS App Board Asana project label May 22, 2024

Bunn commented May 22, 2024

View reviewed changes

...BrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentManagerTests.swift Show resolved Hide resolved

aataraxiaa requested changes May 22, 2024

View reviewed changes

Bunn added 3 commits May 22, 2024 19:07

PR feedback, stop action

5b973bf

PR feedback, final properties

29eea91

Add completion handler for agentFinishedLaunching

3d67be5

Base automatically changed from feature/pir-stabilization to main May 22, 2024 20:16

aataraxiaa requested changes May 23, 2024

View reviewed changes

Merge branch 'main' into bunn/dbp/entitlement

d78c22e

# Conflicts: # DuckDuckGo/Application/AppDelegate.swift # DuckDuckGo/DBP/DataBrokerProtectionSubscriptionEventHandler.swift

Bunn changed the title ~~WIP: checking for entitlement~~ Check for entitlement in DBP agent May 23, 2024

Bunn marked this pull request as ready for review May 23, 2024 09:11

Bunn removed the bot: not in app board Added by automation for pull requests with tasks not added to macOS App Board Asana project label May 23, 2024

Improve tests

42264a4

aataraxiaa approved these changes May 23, 2024

View reviewed changes

Bunn merged commit 2104521 into main May 23, 2024
16 of 19 checks passed

Bunn deleted the bunn/dbp/entitlement branch May 23, 2024 10:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Check for entitlement in DBP agent #2802

Check for entitlement in DBP agent #2802

Bunn commented May 21, 2024 •

edited

Loading

github-actions bot commented May 21, 2024 •

edited

Loading

aataraxiaa left a comment

aataraxiaa May 21, 2024

Bunn May 21, 2024

aataraxiaa left a comment

aataraxiaa left a comment

aataraxiaa left a comment

aataraxiaa May 23, 2024

aataraxiaa left a comment

Check for entitlement in DBP agent #2802

Check for entitlement in DBP agent #2802

Conversation

Bunn commented May 21, 2024 • edited Loading

github-actions bot commented May 21, 2024 • edited Loading

aataraxiaa left a comment

Choose a reason for hiding this comment

aataraxiaa May 21, 2024

Choose a reason for hiding this comment

Bunn May 21, 2024

Choose a reason for hiding this comment

aataraxiaa left a comment

Choose a reason for hiding this comment

aataraxiaa left a comment

Choose a reason for hiding this comment

aataraxiaa left a comment

Choose a reason for hiding this comment

aataraxiaa May 23, 2024

Choose a reason for hiding this comment

aataraxiaa left a comment

Choose a reason for hiding this comment

Bunn commented May 21, 2024 •

edited

Loading

github-actions bot commented May 21, 2024 •

edited

Loading